Skip to content

stream: optimize webstreams pipeTo further#62079

Open
MattiasBuelens wants to merge 5 commits intonodejs:mainfrom
MattiasBuelens:webstreams-optimize-pipeto
Open

stream: optimize webstreams pipeTo further#62079
MattiasBuelens wants to merge 5 commits intonodejs:mainfrom
MattiasBuelens:webstreams-optimize-pipeto

Conversation

@MattiasBuelens
Copy link
Contributor

@MattiasBuelens MattiasBuelens commented Mar 2, 2026

I've started looking more closely at the pipeTo() implementation, and found some low-hanging fruit to further optimize it:

  • We don't have to await writer.ready every time, that's only needed if there is backpressure. So we check that first.
  • We don't have to go through writer.desiredSize to check if there is backpressure, we can read that directly from the internal state of the WritableStreamDefaultController.
  • readableStreamDefaultControllerCallPullIfNeeded already checks state === 'readable' && !closeRequested through readableStreamDefaultControllerCanCloseOrEnqueue, so we don't have to repeat that check.

This also fixes a few more edge cases:

On my machine, this improves the webstreams/pipe-to.js benchmark by 3% to 7%:

Baseline @ 199daab 
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=512 n=500000: 1,639,426.502378644
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=512 n=500000: 1,672,850.078172284
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=512 n=500000: 1,684,703.098034221
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=512 n=500000: 1,658,174.6018059512
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=1024 n=500000: 1,684,320.5922340695
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=1024 n=500000: 1,663,948.3297453062
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=1024 n=500000: 1,651,671.1443471392
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=1024 n=500000: 1,630,100.2576862487
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=2048 n=500000: 1,662,200.8869503932
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=2048 n=500000: 1,638,533.5910855907
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=2048 n=500000: 1,671,184.435256644
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=2048 n=500000: 1,639,392.637815542
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=4096 n=500000: 1,644,874.8168020674
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=4096 n=500000: 1,702,132.8746198711
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=4096 n=500000: 1,608,151.915035545
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=4096 n=500000: 1,688,220.1736233155
This PR @ 7b31401 
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=512 n=500000: 1,768,254.040106831
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=512 n=500000: 1,717,563.2174905646
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=512 n=500000: 1,746,963.515365767
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=512 n=500000: 1,787,073.738236587
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=1024 n=500000: 1,756,757.2790359478
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=1024 n=500000: 1,704,454.421184323
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=1024 n=500000: 1,761,150.6372019118
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=1024 n=500000: 1,766,948.0354718352
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=2048 n=500000: 1,727,179.5537866165
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=2048 n=500000: 1,768,980.3632565797
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=2048 n=500000: 1,752,746.9927243474
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=2048 n=500000: 1,793,850.0362895865
webstreams\pipe-to.js highWaterMarkW=512 highWaterMarkR=4096 n=500000: 1,769,294.5999359516
webstreams\pipe-to.js highWaterMarkW=1024 highWaterMarkR=4096 n=500000: 1,768,876.4768791925
webstreams\pipe-to.js highWaterMarkW=2048 highWaterMarkR=4096 n=500000: 1,748,478.5613798152
webstreams\pipe-to.js highWaterMarkW=4096 highWaterMarkR=4096 n=500000: 1,783,529.4621231847

@nodejs-github-bot nodejs-github-bot added the needs-ci PRs that need a full CI run. label Mar 2, 2026
Credok12
Credok12 reviewed Mar 3, 2026
View reviewed changes
Copy link

@Credok12 Credok12 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok

@MattiasBuelens MattiasBuelens marked this pull request as ready for review March 3, 2026 07:12
@MattiasBuelens MattiasBuelens added the request-ci Add this label to start a Jenkins CI on a PR. label Mar 3, 2026
@github-actions github-actions bot removed the request-ci Add this label to start a Jenkins CI on a PR. label Mar 3, 2026
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Mar 3, 2026

CI: https://ci.nodejs.org/job/node-test-pull-request/71539/

@codecov
Copy link

codecov bot commented Mar 3, 2026 •
edited
Loading

Codecov Report

❌ Patch coverage is 75.00000% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.64%. Comparing base (199daab) to head (7b31401).
⚠️ Report is 81 commits behind head on main.

Files with missing lines Patch % Lines
lib/internal/webstreams/readablestream.js 75.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@            Coverage Diff             @@
##             main   #62079      +/-   ##
==========================================
- Coverage   89.75%   89.64%   -0.11%     
==========================================
  Files         674      676       +2     
  Lines      204886   206310    +1424     
  Branches    39376    39520     +144     
==========================================
+ Hits       183894   184951    +1057     
- Misses      13280    13480     +200     
- Partials     7712     7879     +167
Files with missing lines Coverage Δ
lib/internal/webstreams/readablestream.js 98.34% <75.00%> (-0.12%) ⬇️

... and 166 files with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jasnell jasnell jasnell approved these changes

+1 more reviewer

@Credok12 Credok12 Credok12 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

needs-ci PRs that need a full CI run. web streams

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@MattiasBuelens @nodejs-github-bot @jasnell @Credok12